NO-ISSUE: Fixes KubeletConfig API doc, adds tests and kubebuilder validation by ngopalak-redhat · Pull Request #2651 · openshift/api

ngopalak-redhat · 2026-01-16T05:57:50Z

This PR improves the documentation for the KubeletConfigSpec API to make it more accurate and user-friendly.

Fixes the doc: https://docs.redhat.com/en/documentation/openshift_container_platform/4.20/html/machine_apis/kubeletconfig-machineconfiguration-openshift-io-v1#spec-4

It also adds test coverage similar to the pattern established in #2370.

AutoSizingReserved will default to true from 4.21 onwards. The default value and the description was not documented until now.

Why is this a breaking change?

The additional kubebuilder validation makes this a breaking change.
If the user provides tlsSecurityProfile with just modern or custom fields but omits the type field, the validation passes because !has(self.type) evaluates to true.

Test Coverage

Created test file: machineconfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml

openshift-ci-robot · 2026-01-16T05:57:52Z

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

openshift-ci · 2026-01-16T05:57:53Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

coderabbitai · 2026-01-16T05:57:55Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a YAML test suite for the KubeletConfig CRD covering onCreate and onUpdate scenarios (single-field, multi-field, and invalid-value cases). Adds two optional fields to KubeletConfigSpec: AutoSizingReserved (*bool) and LogLevel (*int32) with validation (0–10). Updates generated Swagger/docs and the CRD: clarifies kubeletConfig and machineConfigPoolSelector descriptions, expands logLevel and tlsSecurityProfile descriptions, and adds x-kubernetes-validations to restrict tlsSecurityProfile to Old and Intermediate.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is directly related to the changeset, explaining the documentation improvements, the new AutoSizingReserved field defaults, and test coverage added.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title accurately describes the main changes: documenting KubeletConfig API, adding tests, and introducing kubebuilder validation rules.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📝 Coding Plan

Generate coding plan for human review comments

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

openshift-ci · 2026-01-16T05:57:55Z

Hello @ngopalak-redhat! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

ngopalak-redhat · 2026-01-16T05:58:03Z

/test all

openshift-ci · 2026-01-16T05:58:05Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign joelspeed for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ngopalak-redhat · 2026-01-16T06:02:25Z

@coderabbitai review

coderabbitai · 2026-01-16T06:02:32Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@payload-manifests/crds/0000_80_machine-config_01_kubeletconfigs.crd.yaml`:
- Around line 118-123: The tlsSecurityProfile description in the KubeletConfig
CRD is inconsistent with the schema: the text claims "Only Old and Intermediate
profiles are supported; maximum minTLSVersion is VersionTLS12" while the schema
allows Modern and Custom (and VersionTLS13) because it relies on
configv1.TLSSecurityProfile which has no kubelet-specific restrictions; to fix,
either add explicit validation to the CRD (e.g., CEL rules or a validating
webhook) to reject profile.type values "Modern" and "Custom" and enforce max
minTLSVersion of "VersionTLS12", or update the tlsSecurityProfile description to
accurately reflect that the schema permits Modern and Custom and VersionTLS13 by
referencing configv1.TLSSecurityProfile semantics; locate and change the
tlsSecurityProfile block in the KubeletConfig CRD (symbol: tlsSecurityProfile,
resource: KubeletConfig) and implement one of the two options consistently
(validation rules if you want to forbid Modern/Custom, otherwise update the
descriptive text).

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 12dcb66 and 8e0b9d6.

⛔ Files ignored due to path filters (2)

machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_kubeletconfigs.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
machineconfiguration/v1/zz_generated.featuregated-crd-manifests/kubeletconfigs.machineconfiguration.openshift.io/AAA_ungated.yaml is excluded by !**/zz_generated.featuregated-crd-manifests/**

📒 Files selected for processing (4)

machineconfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml
machineconfiguration/v1/types.go
machineconfiguration/v1/zz_generated.swagger_doc_generated.go
payload-manifests/crds/0000_80_machine-config_01_kubeletconfigs.crd.yaml

🚧 Files skipped from review as they are similar to previous changes (1)

machineconfiguration/v1/types.go

🔇 Additional comments (2)

machineconfiguration/v1/zz_generated.swagger_doc_generated.go (1)

216-223: Docs read well and are consistent with the CRD updates.
The KubeletConfigSpec descriptions are clear and actionable.

machineconfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml (1)

1-275: Solid coverage for create/update scenarios.
The matrix of single-field and combined updates looks thorough.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

payload-manifests/crds/0000_80_machine-config_01_kubeletconfigs.crd.yaml

openshift-ci-robot · 2026-01-16T06:53:55Z

@ngopalak-redhat: This pull request explicitly references no jira issue.

Details

In response to this:

This PR improves the documentation for the KubeletConfigSpec API to make it more accurate and user-friendly.
The focus is AutoSizingReserved. I have ignored adding the tests and the validation for TLSSecurityProfile. Also I haven't added the kubebuilder validation for LogLevel. They can be done in a different PR as this one focusses on API doc only.

It also adds test coverage similar to the pattern established in #2370.

AutoSizingReserved will default to true from 4.21 onwards. The default value and the description was not documented until now.

Test Coverage

Created test file: machineconfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

ngopalak-redhat · 2026-01-16T06:58:15Z

cc: @haircommander @sairameshv

...onfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_kubeletconfigs.crd.yaml

...nfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml

saschagrunert

Thank you for the PR, I have a bunch of improvements sggested.

Please note that we need to add integration tests for the new validations.

machineconfiguration/v1/types.go

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@machineconfiguration/v1/types.go`:
- Around line 769-774: The CEL XValidation currently permits profiles without an
explicit type and thus allows Modern/Custom via embedded fields; update the
validation on TLSSecurityProfile to require that the type field is present and
equal to either "Old" or "Intermediate" (e.g. change the rule to require
has(self.type) && (self.type == 'Old' || self.type == 'Intermediate')), so
TLSSecurityProfile (the TLSSecurityProfile *configv1.TLSSecurityProfile field)
cannot be set to Modern/Custom without an explicit allowed type.

♻️ Duplicate comments (1)

payload-manifests/crds/0000_80_machine-config_01_kubeletconfigs.crd.yaml (1)

268-271: Keep the CRD validation in sync with the tightened TLS rule.

Same concern as in machineconfiguration/v1/types.go: regenerate this CRD after tightening the CEL rule so missing type can’t bypass the restriction.

🧹 Nitpick comments (1)

machineconfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml (1)

173-220: Add a negative case for tlsSecurityProfile without type.

This will lock in the stricter validation and prevent bypasses if the object is provided without a type.

🧪 Suggested test case

@@
     - name: Should reject tlsSecurityProfile with Custom type
       initial: |
         apiVersion: machineconfiguration.openshift.io/v1
         kind: KubeletConfig
         spec:
           tlsSecurityProfile:
             type: Custom
             custom:
               ciphers:
                 - ECDHE-ECDSA-AES128-GCM-SHA256
               minTLSVersion: VersionTLS12
       expectedError: "only Old and Intermediate TLS profiles are supported for kubelet"
+    - name: Should reject tlsSecurityProfile without type
+      initial: |
+        apiVersion: machineconfiguration.openshift.io/v1
+        kind: KubeletConfig
+        spec:
+          tlsSecurityProfile:
+            custom:
+              ciphers:
+                - ECDHE-ECDSA-AES128-GCM-SHA256
+              minTLSVersion: VersionTLS12
+      expectedError: "only Old and Intermediate TLS profiles are supported for kubelet"

machineconfiguration/v1/types.go

ngopalak-redhat · 2026-01-23T04:02:55Z

Thank you for the PR, I have a bunch of improvements sggested.

Please note that we need to add integration tests for the new validations.

@saschagrunert I made the code changes as suggested. I also added the validations and tests to machineconfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml.

Are these the integration tests you were referring to?

...nfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml

machineconfiguration/v1/types.go

ngopalak-redhat · 2026-01-27T11:40:25Z

@saschagrunert Thanks for the review. I have addressed your comments. Could you take another look? I also replied to the specific discussion here: #2651 (comment)

...nfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml

machineconfiguration/v1/types.go

...nfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml

machineconfiguration/v1/types.go

...nfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml

…PI doc and adds tests

saschagrunert

Thanks for iterating on this. The PR is in good shape. The documentation improvements follow the API conventions well and the ratcheting tests properly cover the upgrade path.

The verify-crdify failure is expected since this PR intentionally tightens logLevel bounds and adds a CEL rule on tlsSecurityProfile. We can /override once the inline comments are addressed.

Putting it into shadow review complete since the required changes are already suggested.

machineconfiguration/v1/types.go

...nfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml

everettraven

A couple comments to add to @saschagrunert 's review but overall this looks pretty good.

machineconfiguration/v1/types.go

...nfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml

machineconfiguration/v1/types.go

ngopalak-redhat · 2026-03-11T04:21:51Z

/hold
There is a bug fix for the log-level in the MCO: openshift/machine-config-operator#5757

ngopalak-redhat · 2026-03-16T10:25:47Z

@saschagrunert @everettraven

Recently TLS 1.3 is supported and hence Modern and Custom profiles also work. So discussed with @bitoku and @asahay19 and made the validation simpler. So removed the racketing test. Note that I'm not calling out that cri-o will inherit this value as its an implementation detail.
The log level is fixed in MCO (NO-ISSUE: Allow setting kubeletconfig log level to zero machine-config-operator#5757), so the validation stays the same here

Please review

payload-manifests/crds/0000_80_machine-config_01_kubeletconfigs.crd.yaml

everettraven · 2026-03-16T12:19:40Z

machineconfiguration/v1/types.go

+	// When specified, the type field can be set to either "Old", "Intermediate", "Modern", "Custom" or omitted for backward compatibility.
+	// maximum minTLSVersion is VersionTLS13.
+	// +kubebuilder:validation:XValidation:rule="!has(self.type) || self.type == 'Old' || self.type == 'Intermediate' || self.type == 'Modern' || self.type == 'Custom'",message="only Old, Intermediate, Modern and Custom TLS profiles are supported for kubelet"


If you are allowing the type to be any of the types that are valid for the TLSSecurityProfile.Type field you probably don't need this validation - the type alias for that field already enforces an enum constraint with these same values.

@everettraven Let's say there is a addition to TLSSecurityProfile.Type enum which kubelet doesn't support yet, then we need this validation right?

In theory, yes. In practice, this is unlikely. Any changes to the TLSSecurityProfile API will need to go through the feature promotion process which will include ensuring that all components either support the changes or they have been explicitly updated to not handle them prior to the promotion of the field.

…PI doc and adds tests

openshift-ci · 2026-03-16T13:39:36Z

@ngopalak-redhat: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/verify-crdify	`2dc542b`	link	true	`/test verify-crdify`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 16, 2026

openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 16, 2026

ngopalak-redhat force-pushed the ngopalak/autonode_doc branch from 12dcb66 to 8e0b9d6 Compare January 16, 2026 06:21

ngopalak-redhat marked this pull request as ready for review January 16, 2026 06:31

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 16, 2026

openshift-ci bot requested review from everettraven and sinnykumari January 16, 2026 06:33

coderabbitai bot reviewed Jan 16, 2026

View reviewed changes

payload-manifests/crds/0000_80_machine-config_01_kubeletconfigs.crd.yaml Show resolved Hide resolved

ngopalak-redhat changed the title ~~Fixes KubeletConfig API doc and adds tests~~ NO-ISSUE: Fixes KubeletConfig API doc and adds tests Jan 16, 2026

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 16, 2026

sairameshv reviewed Jan 19, 2026

View reviewed changes

...onfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_kubeletconfigs.crd.yaml Show resolved Hide resolved

...nfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml Outdated Show resolved Hide resolved

saschagrunert reviewed Jan 22, 2026

View reviewed changes

machineconfiguration/v1/types.go Outdated Show resolved Hide resolved

machineconfiguration/v1/types.go Outdated Show resolved Hide resolved

machineconfiguration/v1/types.go Outdated Show resolved Hide resolved

ngopalak-redhat force-pushed the ngopalak/autonode_doc branch from 8e0b9d6 to b52d6d1 Compare January 22, 2026 14:40

openshift-ci bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 22, 2026

coderabbitai bot reviewed Jan 22, 2026

View reviewed changes

machineconfiguration/v1/types.go Show resolved Hide resolved

ngopalak-redhat force-pushed the ngopalak/autonode_doc branch from b52d6d1 to cb2ecfa Compare January 23, 2026 03:46

saschagrunert reviewed Jan 26, 2026

View reviewed changes

openshift-ci bot removed the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 27, 2026

openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 27, 2026

ngopalak-redhat force-pushed the ngopalak/autonode_doc branch 2 times, most recently from 33fda82 to 1b89d6b Compare January 27, 2026 11:35

saschagrunert reviewed Jan 27, 2026

View reviewed changes

...nfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml Show resolved Hide resolved

ngopalak-redhat force-pushed the ngopalak/autonode_doc branch from d174e51 to abafcac Compare February 19, 2026 05:01

ngopalak-redhat changed the title ~~NO-ISSUE: Fixes KubeletConfig API doc and adds tests~~ NO-ISSUE: Fixes KubeletConfig API doc, adds tests and kubebuilder validation Feb 19, 2026

saschagrunert reviewed Feb 23, 2026

View reviewed changes

kubebuilder validation and clarified API behavior for KubeletConfig A…

13eaa95

…PI doc and adds tests

ngopalak-redhat force-pushed the ngopalak/autonode_doc branch from abafcac to 13eaa95 Compare March 2, 2026 12:54

saschagrunert reviewed Mar 6, 2026

View reviewed changes

machineconfiguration/v1/types.go Outdated Show resolved Hide resolved

...nfiguration/v1/tests/kubeletconfigs.machineconfiguration.openshift.io/KubeletConfigSpec.yaml Show resolved Hide resolved

everettraven reviewed Mar 6, 2026

View reviewed changes

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 11, 2026

ngopalak-redhat force-pushed the ngopalak/autonode_doc branch from 67d61bf to aec4952 Compare March 16, 2026 10:15

bitoku reviewed Mar 16, 2026

View reviewed changes

payload-manifests/crds/0000_80_machine-config_01_kubeletconfigs.crd.yaml Outdated Show resolved Hide resolved

everettraven reviewed Mar 16, 2026

View reviewed changes

ngopalak-redhat force-pushed the ngopalak/autonode_doc branch from aec4952 to 41ea859 Compare March 16, 2026 13:08

kubebuilder validation and clarified API behavior for KubeletConfig A…

2dc542b

…PI doc and adds tests

ngopalak-redhat force-pushed the ngopalak/autonode_doc branch from 41ea859 to 2dc542b Compare March 16, 2026 13:10

Conversation

ngopalak-redhat commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Jan 16, 2026

Uh oh!

openshift-ci bot commented Jan 16, 2026

Uh oh!

coderabbitai bot commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Uh oh!

openshift-ci bot commented Jan 16, 2026

Uh oh!

ngopalak-redhat commented Jan 16, 2026

Uh oh!

openshift-ci bot commented Jan 16, 2026

Uh oh!

ngopalak-redhat commented Jan 16, 2026

Uh oh!

coderabbitai bot commented Jan 16, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openshift-ci-robot commented Jan 16, 2026

Uh oh!

ngopalak-redhat commented Jan 16, 2026

Uh oh!

Uh oh!

Uh oh!

saschagrunert left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ngopalak-redhat commented Jan 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ngopalak-redhat commented Jan 27, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

saschagrunert left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

everettraven left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ngopalak-redhat commented Mar 11, 2026

Uh oh!

ngopalak-redhat commented Mar 16, 2026

Uh oh!

ngopalak-redhat commented Jan 16, 2026 •

edited

Loading

coderabbitai bot commented Jan 16, 2026 •

edited

Loading

saschagrunert left a comment •

edited

Loading